Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

fix(commands/changelog): use topological order for commit ordering #677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 11, 2023
Merged

fix(commands/changelog): use topological order for commit ordering #677

merged 1 commit into from
Mar 11, 2023

Conversation

sacha-c
Copy link
Contributor

@sacha-c sacha-c commented Mar 1, 2023 •
edited
Loading

Description

This MR comes from an issue raised here

Topological ordering should be used when ordering commits in changelog history. This allows commits to be shown properly in the order they were added to the codebase, even if non-linear merges were used

Checklist

  • Add test cases to all the changes you introduce
  • Run ./scripts/format and ./scripts/test locally to ensure this change passes linter check and test
  • Test the changes on the local machine manually
  • Update the documentation for the changes
    • No documentation to update

Expected behavior

With a git history that has non-linear merges, I want my commits to be ordered in the order that they were introduced in the codebase.

So having a git graph like so:

* feat: I will be merged first - (2023-03-01 11:35:51 +0100) |  (branchB)
| * feat: I will be merged second - (2023-03-01 11:35:22 +0100) |  (branchA)
|/
* feat: initial commit - (2023-03-01 11:34:54 +0100) |  (HEAD -> main)

If I merge branchB before branchA like so:

*   Merge branch 'branchA' - (2023-03-01 11:42:59 +0100) |  (HEAD -> main)
|\
| * feat: I will be merged second - (2023-03-01 11:35:22 +0100) |  (branchA)
* | feat: I will be merged first - (2023-03-01 11:35:51 +0100) |  (branchB)
|/
* feat: initial commit - (2023-03-01 11:34:54 +0100) |

I expect the generated changelog to be this:

## Unreleased

### Feat
- I will be merged second
- I will be merged first
- initial commit

I also expect commands like cz changelog v0.3.0 and cz changelog v0.2.0..v0.3.0 to produce identical changelogs for v.0.3.0 (which isn't currently the case - see issue)

Steps to Test This Pull Request

  1. Initialize an empty repository
  2. Create an initial commit "feat: initial commit`
  3. Create a new branch branchA
  4. Create a new branch branchB
  5. Switch to branchA
  6. Commit a change feat: I will be merged second
  7. Switch to branchB
  8. Commit a change feat: I will be merged first
  9. Switch to main
  10. Merge branchB into main
  11. Merge branchA into main
  12. Generate a changelog cz changelog --dry-run
  13. See that the changes are ordered by their introduction in the codebase
## Unreleased

### Feat
- I will be merged second
- I will be merged first
- initial commit

Additional context

@sacha-c sacha-c force-pushed the fix/changelog-commit-order branch from 3f20fb7 to 4ff1568 Compare March 1, 2023 13:04
@codecov
Copy link

codecov bot commented Mar 1, 2023 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (v3@9b5f311). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files 
@@          Coverage Diff          @@
##             v3     #677   +/-   ##
=====================================
  Coverage      ?   97.98%           
=====================================
  Files         ?       41           
  Lines         ?     1835           
  Branches      ?        0           
=====================================
  Hits          ?     1798           
  Misses        ?       37           
  Partials      ?        0
Flag Coverage Δ
unittests 97.98% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sacha-c
fix(commands/changelog): use topological order for commit ordering
b5125cb 
Topological ordering should be used when ordering commits in changelog history.
This allows commits to be shown properly in the order they were added to the codebase,
even if non-linear merges were used
@sacha-c sacha-c force-pushed the fix/changelog-commit-order branch from 4ff1568 to b5125cb Compare March 6, 2023 21:36
@sacha-c sacha-c mentioned this pull request Mar 8, 2023
Closed
@woile woile merged commit 642f2c8 into commitizen-tools:v3 Mar 11, 2023
@woile
Copy link
Member

woile commented Apr 17, 2023

I'm testing this feature and something weird is happening:

Run cz changelog 2.32.1 --dry-run and we get:

## v2.32.1 (2022-08-21)

### Refactor

- **git**: test the git log parser behaves properly when the repository has no commits
- **changelog**: fixes logic issue made evident by latest fix(git) commit

### Fix

- **git**: Improves error checking in get_tags
- **git**: improves git error checking in get_commits

And when we run a range cz changelog 2.32.0..2.32.1 --dry-run the previous tag is empty:

## v2.32.1 (2022-08-21)

## v2.32.0 (2022-08-21)

### Refactor

- **git**: test the git log parser behaves properly when the repository has no commits
- **changelog**: fixes logic issue made evident by latest fix(git) commit

### Fix

- **git**: Improves error checking in get_tags
- **git**: improves git error checking in get_commits

### Feat

- **pre-commit**: Add commitizen-branch hook

Thoughts?

@woile
Copy link
Member

woile commented Apr 17, 2023

Forget my comment, I was testing with an old version 🤦🏻

@sacha-c
Copy link
Contributor Author

sacha-c commented Apr 17, 2023

Nice! Happy to hear this is being tested 🙂

@sacha-c sacha-c deleted the fix/changelog-commit-order branch April 17, 2023 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sacha-c @woile